feat(stack): typed external OAuth providers and redirect allow list#5816
feat(stack): typed external OAuth providers and redirect allow list#5816martijnwalraven wants to merge 3 commits into
Conversation
Ports the classic CLI's [auth.external.*] and additional_redirect_urls surfaces to the stack — the auth config the Go CLI has that the stack didn't. AuthConfig gains `external`, a record of typed per-provider configs (enabled default-true, clientId, optional secret, redirectUri defaulted to the issuer-derived <externalUrl>/auth/v1/callback, url, skipNonceCheck, emailOptional), translated to GOTRUE_EXTERNAL_<ID>_* env in the auth member exactly like the classic CLI's start path — GoTrue itself validates provider ids — and `additionalRedirectUrls`, translated to GOTRUE_URI_ALLOW_LIST the same way. Booleans and the allow list emit explicitly, defaults included: native-mode spawns extend the parent environment, so an omitted variable would let the shell override the typed config (start.go shadows the same way with %t and an always-appended allow list). Typed config over a raw env valve deliberately: the stack's config surfaces are typed camelCase interfaces, and a caller-wins env record could silently override core wiring (DB URL, JWT secret) — the rejected shape. Contract tests cover both factory shapes, the issuer-derived redirect default, the secretless-provider case, the allow-list translation and its empty-but-present baseline, and the explicit boolean defaults. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eda79bd1c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; | ||
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri ?? `${opts.externalUrl}/auth/v1/callback`; |
There was a problem hiding this comment.
Treat empty redirectUri as unset
When config is translated from the existing Supabase auth config, an omitted provider redirect is represented as the empty string (redirect_uri = "" in apps/cli/src/shared/init/project-init.templates.ts:327-328), and the legacy launcher falls back when the string is empty (apps/cli-go/internal/start/start.go:1438-1441). This ?? only falls back for null/undefined, so redirectUri: "" emits GOTRUE_EXTERNAL_<ID>_REDIRECT_URI=; for enabled providers that rely on the default callback, local OAuth sign-in will use a blank callback instead of the derived URL. Please treat "" as unset before emitting the env var.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 5ee3902. ?? only coalesced null/undefined, so a decoded config's redirect_uri = "" (the TOML surface can't say "absent"; the config package's provider schema defaults the key to "") would have emitted an empty GOTRUE_EXTERNAL_<ID>_REDIRECT_URI, which GoTrue's ValidateOAuth refuses with "missing redirect URI". Empty now falls back to the issuer-derived callback, exactly like start.go, with a test row for the empty-string case.
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri ?? `${opts.externalUrl}/auth/v1/callback`; | ||
| env[`${prefix}_SKIP_NONCE_CHECK`] = String(provider.skipNonceCheck ?? false); | ||
| env[`${prefix}_EMAIL_OPTIONAL`] = String(provider.emailOptional ?? false); | ||
| if (provider.url !== undefined) { |
There was a problem hiding this comment.
When the existing config surface represents an omitted provider URL as url = "" (apps/cli/src/shared/init/project-init.templates.ts:329-331), the legacy launcher intentionally skips the env var unless it is non-empty (apps/cli-go/internal/start/start.go:1451-1452). This condition emits GOTRUE_EXTERNAL_<ID>_URL= for an empty string, which can override GoTrue's built-in provider defaults (for example GitLab's default URL) and break enabled providers that rely on those defaults. Please only emit the URL when it is non-empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5ee3902, though the failure mode is narrower than described: GoTrue reads an empty external URL as unset — every provider routes it through chooseHost(base, default), which falls back to the built-in default when the value is empty, and an empty-set env var is indistinguishable from an absent one to its config loader — so the empty GOTRUE_EXTERNAL_<ID>_URL= couldn't override GitLab's default. We also went the other way from the suggestion: the var now always emits, empty included, rather than matching start.go's skip-when-empty. This stack spawns native services by extending the parent environment, so omitting the var would let a stray shell GOTRUE_EXTERNAL_<ID>_URL supply the provider URL — the same reason the booleans and allow list already emit at their defaults. Behaviorally identical on the GoTrue side; start.go can afford the skip because its containers never inherit a shell.
The classic surface can't say "absent": TOML strings decode to "" — the generated config template ships redirect_uri = "" and url = "", and the config package's provider schema defaults both keys to "" — so start.go substitutes the issuer-derived callback when RedirectUri is empty. The stack translation coalesced only null/undefined, letting an empty redirectUri emit GOTRUE_EXTERNAL_<ID>_REDIRECT_URI= and break that provider's OAuth flow (GoTrue's ValidateOAuth refuses with "missing redirect URI"). Empty now falls back to the derived callback, like start.go. url goes the other way: it now always emits, empty included, instead of matching start.go's skip-when-empty. GoTrue reads an empty URL as unset (chooseHost falls back to each provider's default), so the emitted empty is behaviorally identical to the skip — and native-mode spawns extend the parent environment, so omission would let a shell GOTRUE_EXTERNAL_<ID>_URL supply the provider URL. start.go can afford the skip: its containers never inherit a shell. Addresses the Codex review findings on the PR thread. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ee3902c97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; | ||
| env[`${prefix}_REDIRECT_URI`] = provider.redirectUri || `${opts.externalUrl}/auth/v1/callback`; |
There was a problem hiding this comment.
Preserve auth external URLs when deriving callbacks
Existing callers can set auth.externalUrl to the Auth URL itself (the public API documents it as “Auth external URL” in packages/stack/README.md:116, and this value is passed through to GoTrue as API_EXTERNAL_URL). In that configuration, e.g. https://api.example.com/auth/v1, this fallback emits GOTRUE_EXTERNAL_GOOGLE_REDIRECT_URI=https://api.example.com/auth/v1/auth/v1/callback, so OAuth providers reject the callback mismatch whenever redirectUri is left unset. Derive from a normalized auth URL, or only append /auth/v1 when the configured value is the API root.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8aa9ad2, at the root rather than by normalizing: externalUrl was carrying two semantics at once. Its name and README description match the classic CLI's auth.external_url — the with-path URL (GetExternalURL derives <api-root>/auth/v1) — but its default was the bare API root, and it feeds API_EXTERNAL_URL, which GoTrue treats as the with-path auth URL: it's the external host outgoing URLs are built against (middleware's external-host fallback), and loadCustomProvider derives <it>/callback for custom providers. The bare root there was itself a latent defect — the stack's proxy only routes /auth/v1/*, so GoTrue-built absolute URLs pointed at paths the proxy never serves. The field is now pinned to its name: the default gains /auth/v1 and the provider callback derives as ${externalUrl}/callback, start.go's JwtIssuer + "/callback" exactly. Your https://api.example.com/auth/v1 example now yields …/auth/v1/callback, the default-config redirect value is unchanged, and API_EXTERNAL_URL gains the classic-parity value. Sniffing for an existing /auth/v1 suffix would have kept the ambiguity alive.
| const prefix = `GOTRUE_EXTERNAL_${id.toUpperCase()}`; | ||
| env[`${prefix}_ENABLED`] = String(provider.enabled ?? true); | ||
| env[`${prefix}_CLIENT_ID`] = provider.clientId; | ||
| env[`${prefix}_SECRET`] = provider.secret ?? ""; |
There was a problem hiding this comment.
Require secrets before enabling OAuth providers
When callers rely on the new optional secret field, enabled still defaults to true, so auth: { external: { google: { clientId } } } starts GoTrue with GOTRUE_EXTERNAL_GOOGLE_ENABLED=true and an empty secret. The current auth service used by the stack rejects enabled OAuth providers with an empty secret in ValidateOAuth, so the advertised secretless provider path fails at /authorize; require a secret for enabled providers or avoid enabling them when it is omitted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This one matches GoTrue's intended model, so the surface stays as is (with a sharper doc comment in 8aa9ad2): the secretless path isn't /authorize — it's the id-token grant (google one-tap, sign-in with apple), which reads the provider config directly in token_oidc.go and never calls ValidateOAuth, so no secret is needed there. That's also why the config package's own schema exempts exactly apple and google from its enabled-requires-secret check, and the classic CLI emits an empty secret the same way. For flows that do need a secret, GoTrue refuses loudly at /authorize ("missing OAuth secret") — identical to classic behavior. Requiring a secret here would break id-token-only configurations upstream itself supports, and auto-disabling would silently turn off a provider the caller declared; provider-specific validation stays with the config layer (which has it) and GoTrue (which enforces it at request time).
… from it
The PR review surfaced a double-append: with externalUrl set to the
auth URL itself, the provider redirect default emitted
<url>/auth/v1/auth/v1/callback. The root cause was a two-faced field,
not the derivation: externalUrl is named and documented as the
classic CLI's auth.external_url — the WITH-path URL (GetExternalURL
derives <api-root>/auth/v1) — but defaulted to the bare API root, and
it feeds API_EXTERNAL_URL, which GoTrue treats as the external host
it builds outgoing URLs against (mail links via middleware.go's
external-host fallback, custom-provider callbacks in
loadCustomProvider). The bare root there was a latent defect: the
stack's proxy only routes /auth/v1/*, so GoTrue-built absolute URLs
pointed at paths the proxy never serves.
externalUrl now means what its name says: the auth service's public
URL through the gateway, path included, defaulting to
<api-root>/auth/v1. The provider callback derives as
${externalUrl}/callback — start.go's JwtIssuer + "/callback" exactly
— so the emitted REDIRECT_URI at defaults is unchanged, and
API_EXTERNAL_URL gains the classic-parity value.
Also sharpens the provider secret doc: the id-token flow (google
one-tap, apple sign-in) needs no secret — GoTrue's id_token grant
never calls ValidateOAuth — while /authorize refuses without one at
request time, which is why the config schema exempts exactly apple
and google from its enabled-requires-secret check. Documents the new
AuthConfig fields in the README alongside the re-pinned externalUrl
row.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Problem
The stack has no surface for GoTrue's external OAuth providers or extra redirect URLs — the
[auth.external.*]andadditional_redirect_urlsconfig the classic CLI translates intoGOTRUE_EXTERNAL_*/GOTRUE_URI_ALLOW_LISTenv (start.go) has no equivalent onAuthConfig, so a programmatic embedder can't wire e.g. Google login. (Use case: a dev-environment supervisor embedding the stack, with Google OAuth in its dev profile.)Change
AuthConfiggains:external— a record of typed per-provider configs keyed by GoTrue provider id (enableddefault-true,clientId, optionalsecret,redirectUridefaulting to the issuer-derived<externalUrl>/auth/v1/callback,url,skipNonceCheck,emailOptional), translated toGOTRUE_EXTERNAL_<ID>_*in the auth member exactly like the classic CLI's start path; GoTrue itself validates provider ids.additionalRedirectUrls— translated toGOTRUE_URI_ALLOW_LISTthe same way.The booleans and the allow list emit explicitly, defaults included: native-mode spawns extend the parent environment (
extendEnv: true), so an omitted variable would let a same-named shell variable override the typed config —start.goshadows the same way (%tbooleans, always-appended allow list).Considered and rejected
auth.envpassthrough: caller-wins env could silently override core wiring (GOTRUE_DB_DATABASE_URL,GOTRUE_JWT_SECRET), and the stack's config surfaces are typed camelCase interfaces.@supabase/config's provider schema types directly: those are the toml layer's snake_case shapes; the stack keeps its own camelCase config interfaces like the rest ofAuthConfig.Tests
Contract tests on both factory shapes: the issuer-derived redirect default, explicit fields reaching docker args, the secretless-provider case (empty emit), the allow-list translation and its empty-but-present baseline, and explicit boolean defaults.
🤖 Generated with Claude Code